-
Notifications
You must be signed in to change notification settings - Fork 688
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Regarding issue #6468 (tagreader problems) #6903
base: master
Are you sure you want to change the base?
Regarding issue #6468 (tagreader problems) #6903
Conversation
When using Apple style Mp4 tags the tmpo property which normally is translated to BPM is not fetched from audio file metadata. This adds this feature without fixing other problems, e.g. when changing tags within clementine existing but unchanged tags become deranged. I strongly suggest to change taglib interface in a way kid3 is using (explicit adding/removing tags). The minor change within utilities is just to avoid searching for a file within huge direcories as simply showing directory without positioning or at least highlighting is unuseable.
…dded to allow single tag editting while the original method SaveFile was overtwriting existing tags with invalid data. Therefor the original method now only effects basic attributes as required e.g. after ripping.
in single tag mode. This avoids to overwrite uneffected properties within audiofiles as it occured when using SaveFile interface. Quite sure not all names are used for tagging, only those marked edittable can be updated/inserted/removed. The names are taken from taglib translations when available (by convention written in uppercase allthough taglib name mapping is caseinsensitive). Main source: 3rdparty/taglib/.../id3v2frame translations. Names written lowercase miss a taglib equivalent and are used for internal informations. Note: When adding lines to column enumeration: Try to add mappable taglib properties.
When using Apple style Mp4 tags the tmpo property which normally is translated to BPM is not fetched from audio file metadata. This adds this feature without fixing other problems, e.g. when changing tags within clementine existing but unchanged tags become deranged. I strongly suggest to change taglib interface in a way kid3 is using (explicit adding/removing tags). The minor change within utilities is just to avoid searching for a file within huge direcories as simply showing directory without positioning or at least highlighting is unuseable.
…dded to allow single tag editting while the original method SaveFile was overtwriting existing tags with invalid data. Therefor the original method now only effects basic attributes as required e.g. after ripping.
in single tag mode. This avoids to overwrite uneffected properties within audiofiles as it occured when using SaveFile interface. Quite sure not all names are used for tagging, only those marked edittable can be updated/inserted/removed. The names are taken from taglib translations when available (by convention written in uppercase allthough taglib name mapping is caseinsensitive). Main source: 3rdparty/taglib/.../id3v2frame translations. Names written lowercase miss a taglib equivalent and are used for internal informations. Note: When adding lines to column enumeration: Try to add mappable taglib properties.
…gLog(Debug) removed" This reverts commit adc1fab.
mp4 tag 'tmpo'. Main change: Instead of posting whole song properties to external tagreader only single tag (aka the one just editted) is posted. That required a new tagreader interface. As a side effect the tag translation which differs depending on audio file type (mp3, aac, flac, ...) is delegated to propertymap in 3rdparty tagreader. On the other hand tagname are made available to playlist class (matching the columns enumeration) which should provide an easier way to add additional filepersistent tags.
You mentioned that you're new to git, so here are some quick squash instructions that I posted in a different PR recently: #6887 (comment) |
<< "MOOD" | ||
<< "performer" | ||
<< "GROUPING" | ||
<< "ORIGINALDATE"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A static GetTagName method with a case statement is probably less error prone.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
QStringList Song::kColumns was taken as example. QStringList might cause more runtime overhead but readability is much better to name enumeration items.
if (ret) { | ||
// Linux: inotify doesn't seem to notice the change to the file unless we | ||
// change the timestamps as well. (this is what touch does) | ||
utimensat(0, QFile::encodeName(filename).constData(), nullptr, 0); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure why that would be. The Qt code that handles the inotify specifies the IN_MODIFY flag. And the modification of the file would be updating the timestamp as well. Not sure what games taglib would be playing to circumvent this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's just a copy from former code. I would like to remove that.
External tagreader adaptions
to enhance usage of playlist for tag editting including fix to get BPM from mp4 tag 'tmpo'.
Main change: Instead of posting whole song properties to external tagreader
only single tag (aka the one just editted) is posted. That required a new
tagreader interface and changes on whole path from ext to playlist.
As a side effect the tag translation which differs depending on
audio file type (mp3, aac, flac, ...) is delegated to propertymap in
3rdparty tagreader. On the other hand tagnames are made available to
playlist class (matching the columns enumeration) which should provide an
easier way to add additional filepersistence tags.
Sorry for huge commit list (some are simple reverts) but I'm new to git.